ENG-3301: Add threading header support to email providers (Story 2) by JadeCara · Pull Request #8122 · ethyca/fides

JadeCara · 2026-05-06T17:47:35Z

Description Of Changes

Adds threading header support to all 4 email providers. Extends EmailForActionType with optional reply_to, message_id, in_reply_to, references, and body_text fields. All fields default to None — existing callers are unaffected (backward compatible).

Switches AWS SES from send_email() (simple API) to send_raw_email() with MIME construction via email.message.EmailMessage for custom header support.

Code Changes

Extended EmailForActionType schema with 5 optional threading/envelope fields
Mailgun: maps to h:Reply-To, h:Message-ID, h:In-Reply-To, h:References, text
Twilio/SendGrid: maps to ReplyTo, Header objects, Content("text/plain", ...)
Mailchimp: maps to message.headers dict, message.text
AWS SES: switched to send_raw_email() with EmailMessage MIME construction
Updated SESClient type stub: send_email() → send_raw_email()
New test_provider_headers.py — per-provider header mapping tests (headers present + absent)
New test_provider_map.py — _PROVIDER_MAP completeness invariant test
Non-ASCII encoding tests for SES MIME (subjects + bodies)

Steps to Confirm

1. Server starts cleanly

docker logs fides 2>&1 | grep -i traceback

Confirm no import errors from the new/updated messaging provider modules.

2. Backward compatibility — send a test email

Set up a Mailgun config (or use whichever provider you have credentials for) and send a test message. Since no callers populate the new threading fields yet, this should behave identically to before the PR:

PUT /api/v1/messaging/default/mailgun
{"service_type": "mailgun", "details": {"domain": "<your-domain>", "api_version": "v3", "is_eu_domain": false}}

PUT /api/v1/messaging/default/mailgun/secret
{"mailgun_api_key": "<your-key>"}

POST /api/v1/messaging/test/mailgun
{"email": "<your-email>"}

Verify 200 response and email arrives. Open the received email's raw source (Gmail: ⋮ → "Show original") and confirm there are no In-Reply-To, References, or unexpected Reply-To headers — the new fields default to None and should be omitted entirely.

3. SES MIME migration

This is the riskiest change — SES now builds a full MIME message via email.message.EmailMessage and sends it with send_raw_email() instead of the simple send_email() API. Set up an SES config:

PUT /api/v1/messaging/default/aws_ses
{"service_type": "aws_ses", "details": {"aws_region": "<region>", "email_from": "<verified-email>", "domain": "<verified-domain>"}}

PUT /api/v1/messaging/default/aws_ses/secret
{"auth_method": "secret_keys", "aws_access_key_id": "<key>", "aws_secret_access_key": "<secret>"}

POST /api/v1/messaging/test/aws_ses
{"email": "<your-email>"}

Verify 200 response and email arrives. Open the raw source and confirm:

Content-Type is text/html (single-part, since body_text is not set)
From, To, Subject headers are present and correct
No malformed MIME boundaries or encoding issues

4. Threading header coverage (automated)

The new threading fields (reply_to, message_id, in_reply_to, references, body_text) can't be exercised via the test message API — no caller populates them yet. That's intentional; this PR lays infrastructure for ENG-3299.

test_provider_headers.py covers all 4 email providers with two tests each:

headers included: sets all 5 fields, asserts each provider maps them to its provider-specific format (e.g., Mailgun h:Reply-To, SendGrid Header objects, Mailchimp message.headers dict, SES MIME headers)
headers omitted: leaves all 5 fields as None, asserts no extra headers appear in the outgoing request/message

Additionally, SES has two encoding tests confirming non-ASCII subjects and bodies survive the MIME round-trip.

To run locally:

docker exec fides bash -c "pytest --no-cov tests/service/messaging/test_provider_headers.py -v"

Pre-Merge Checklist

Issue requirements met
All CI pipelines succeeded
CHANGELOG.md updated
- Updates unreleased work already in Changelog, no new entry necessary
UX feedback:
- No UX review needed
Followup issues:
- Followup issues created
Database migrations:
- No migrations
Documentation:
- No documentation updates required

🤖 Generated with Claude Code

Introduce BaseMessageProviderService / BaseEmailProviderService / BaseSMSProviderService ABCs and migrate Mailgun, TwilioEmail, MailchimpTransactional, and TwilioSMS dispatchers to provider classes. AWS SES remains on the legacy dispatcher and will be migrated in a follow-up PR. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

vercel · 2026-05-06T17:47:41Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
fides-plus-nightly	Ready	Preview, Comment	May 12, 2026 4:20pm

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
fides-privacy-center	Ignored		May 12, 2026 4:20pm

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

SES is intentionally excluded from _resolve_provider_map() and PROVIDER_MAP since it uses the legacy dispatcher until PR2. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Extract AwsSesService from the legacy aws_ses_service module into the new messaging_providers package. Adds SES identity validation on config save, removes the legacy _aws_ses_dispatcher fallback, and deletes the old src/fides/service/messaging/aws_ses_service.py module. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The old fides.service.messaging.aws_ses_service module is deleted in this PR, so the import must be removed. Also updates comments now that SES is in the provider map. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Extend EmailForActionType with optional reply_to, message_id, in_reply_to, references, and body_text fields. Map these to provider-specific APIs in all 4 email providers: - Mailgun: h:Reply-To, h:Message-ID, etc. + text field - Twilio/SendGrid: ReplyTo, Header objects + Content text/plain - Mailchimp: message.headers dict + message.text - AWS SES: switch to send_raw_email with MIME construction via email.message.EmailMessage for custom header support Add PROVIDER_MAP completeness invariant test and per-provider header mapping tests including non-ASCII encoding coverage. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Resolve merge conflicts between story 2 (threading headers, MIME-based SES sending) and story 1 base refactor (provider helpers, error handling, validate_on_save). Adopts main's infrastructure, preserves story 2 features. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

codecov · 2026-05-11T18:07:01Z

Codecov Report

❌ Patch coverage is 98.21429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.46%. Comparing base (c82abbc) to head (1952a2c).

Files with missing lines	Patch %	Lines
...e/messaging/messaging_providers/aws_ses_service.py	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8122      +/-   ##
==========================================
+ Coverage   85.45%   85.46%   +0.01%     
==========================================
  Files         650      650              
  Lines       42363    42416      +53     
  Branches     4971     4980       +9     
==========================================
+ Hits        36201    36252      +51     
- Misses       5054     5055       +1     
- Partials     1108     1109       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

JadeCara · 2026-05-11T18:48:51Z

/code-review

claude

Code Review: PR #8122 — Threading header support for email providers

Good overall approach — the abstraction is clean and the per-provider mapping is well-structured. Two latent correctness bugs should be addressed before any caller populates the new fields (ENG-3299), plus a few minor suggestions.

Bugs (latent — no caller sets `body_text` or threading fields yet)

1. SES: EmailMessage() without policy=email.policy.SMTP will produce UTF-8 headers that SES classic rejects for non-ASCII subjects

EmailMessage() with no policy defaults to EmailPolicy (RFC 6532 / SMTPUTF8), which writes non-ASCII header values as raw UTF-8. SES classic's send_raw_email expects RFC 2822 with RFC 2047 encoded-word encoding. The test_non_ascii_subject_encoding test only round-trips through Python and won't catch this SES rejection. See inline comment on aws_ses_service.py:156.

2. Twilio/SendGrid: add_content(text/plain) appends plain text after HTML, inverting RFC 2046 preference order

_compose_mail initializes Mail with Content("text/html", ...) first, then add_content(Content("text/plain", ...)) appends after it. In multipart/alternative, the last part is the most preferred — so clients that respect RFC 2046 will render plain text instead of HTML. See inline comment on twilio_email_service.py:67.

Minor Suggestions

mailchimp_transactional_service.py:39: Mandrill has a native reply_to message field; setting it via headers["Reply-To"] works but is non-canonical. Minor correctness concern.
aws_ses_service.py:35: Protocol stub return type dict[str, str] is inaccurate — should be dict[str, Any] to match the actual boto3 response structure.
messaging.py:315: New fields use str | None while the rest of EmailForActionType uses Optional[str]. Minor style inconsistency worth making consistent.

Positives

The _PROVIDER_MAP completeness test (test_provider_map.py) is a great invariant — it'll catch missing providers automatically as new MessagingServiceType values are added.
Per-provider tests cover both the "headers included" and "headers omitted" cases cleanly.
The except MessageDispatchException: raise guard in send_email prevents double-wrapping of already-typed exceptions — good catch.
Backward compatibility is solid: all new fields default to None and are additive to the schema.

🔬 Codegraph: unavailable

💡 Write /code-review in a comment to re-run this review.

- SES: add policy=email_policy.SMTP for RFC 2047 non-ASCII header encoding - Twilio/SendGrid: fix RFC 2046 content ordering (text/plain before text/html) - Mailchimp: use Mandrill's native reply_to field instead of headers dict - SES: fix Protocol stub return type to dict[str, Any] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Move body_text handling into _compose_mail to build the content list in correct RFC 2046 order from the start, using the content setter (which appends) instead of assigning to contents (which has no setter). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Test was asserting reply_to in headers dict, but we moved it to Mandrill's native msg_payload["reply_to"] field in the review feedback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

erosselli · 2026-05-11T20:28:33Z

+    message_id: str | None = None  # RFC 5322 Message-ID
+    in_reply_to: str | None = None
+    references: str | None = None
+    body_text: str | None = None  # plaintext alternative


is body HTML then? maybe a comment next to it would also help clarify the difference between body and body_text

Done — added inline comments clarifying body is HTML and body_text is the plaintext alternative for multipart/alternative.

erosselli · 2026-05-11T20:32:26Z

+
+    @staticmethod
+    def _build_mime(
+        from_address: str, to: str, message: EmailForActionType


is there anything AWS SES specific here? if not can we make this a method on the base class?

Moved _build_mime to BaseEmailProviderService.build_mime() so any MIME-based provider (SES, future SMTP/Gmail) can reuse it. Also extracted get_threading_headers(message, header_prefix) on the base class — all four providers now use it instead of duplicating the header logic.

erosselli · 2026-05-11T20:34:01Z

+        msg_payload: dict = {
+            "from_email": self.from_email,
+            "subject": message.subject,
+            "html": message.body,
+            "to": [{"email": to.strip(), "type": "to"}],
+        }
+
+        # Reply-to uses Mandrill's native field
+        if message.reply_to:
+            msg_payload["reply_to"] = message.reply_to
+
+        # Threading headers go in the headers dict (no native Mandrill equivalents)
+        headers = {}
+        if message.message_id:
+            headers["Message-ID"] = message.message_id
+        if message.in_reply_to:
+            headers["In-Reply-To"] = message.in_reply_to
+        if message.references:
+            headers["References"] = message.references
+        if headers:
+            msg_payload["headers"] = headers
+        if message.body_text:
+            msg_payload["text"] = message.body_text


can we not use _build_mime here? if not, is there common logic we could extract? e.g the headers

def get_headers(message): headers = {} if message.reply_to: headers["Reply-To"] = message.reply_to if message.message_id: headers["Message-ID"] = message.message_id if message.in_reply_to: headers["In-Reply-To"] = message.in_reply_to if message.references: headers["References"] = message.references return headers

Done — extracted get_threading_headers() on BaseEmailProviderService. Mailchimp uses it for the headers dict; reply_to stays on the native Mandrill field.

erosselli · 2026-05-11T20:36:23Z

+            if message.reply_to:
+                data["h:Reply-To"] = message.reply_to
+            if message.message_id:
+                data["h:Message-ID"] = message.message_id
+            if message.in_reply_to:
+                data["h:In-Reply-To"] = message.in_reply_to
+            if message.references:
+                data["h:References"] = message.references


okay -- maybe get_headers(message, header_prefix = None) so we can do something like:

def get_headers(message, header_prefix): headers = {} prefix = "" if header_prefix: prefix = f"{header_prefix}:" if message.reply_to: headers[f"{prefix}Reply-To"] = message.reply_to if message.message_id: headers[f"{prefix}Message-ID"] = message.message_id if message.in_reply_to: headers[f"{prefix}In-Reply-To"] = message.in_reply_to if message.references: headers[f"{prefix}References"] = message.references return headers

Done — Mailgun uses self.get_threading_headers(message, header_prefix='h:') to get prefixed keys. All providers now share the same helper.

Co-authored-by: Eliana Rosselli <67162025+erosselli@users.noreply.github.com>

…rvice.py Co-authored-by: Eliana Rosselli <67162025+erosselli@users.noreply.github.com>

- Clarify body (HTML) vs body_text (plaintext) with inline comments - Extract get_threading_headers() and build_mime() to BaseEmailProviderService - All four providers now use shared header helper instead of duplicating logic - Mailgun uses header_prefix="h:" for prefixed keys - Mailchimp keeps native reply_to field, uses helper for remaining headers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

erosselli

approved with some comments

erosselli · 2026-05-12T12:46:50Z

+        if message.body_text:
+            msg.set_content(message.body_text)
+            msg.add_alternative(message.body, subtype="html")
+        else:
+            msg.set_content(message.body, subtype="html")


Maybe we should include a commen on why we prefer raw text over html?

Done — added a comment explaining the RFC 2046 multipart/alternative ordering convention.

erosselli · 2026-05-12T12:47:51Z

+
+        # Threading headers (exclude Reply-To — handled natively above)
+        threading_headers = self.get_threading_headers(message)
+        threading_headers.pop("Reply-To", None)


I think we should use an enum or at least constants for the header names ? e.g it'd be easy to mess up and write reply-to or Reply-to etc.

Done — extracted header name constants (HEADER_REPLY_TO, HEADER_MESSAGE_ID, etc.) on BaseEmailProviderService and updated all provider references (here, twilio_email_service.py, and get_threading_headers()).

erosselli · 2026-05-12T12:54:34Z

+            mail.content = Content("text/plain", body_text)
+            mail.content = Content("text/html", message_body)


does this actually append both contents?

Yes — the Mail.content setter appends (calls add_content() internally), so both parts are included. This builds a multipart/alternative with text/plain and text/html, same as build_mime() in the base class.

If body_text isn't populated, it just sends HTML like before — no behavior change. The plaintext path is for recipients whose email client or server prefers plain text — Fides just provides both options in the envelope so whatever's on the other end can pick. Examples:

Accessibility-focused email setups

Corporate environments with strict HTML filtering

Plain-text-preferred mail clients

erosselli · 2026-05-12T12:55:12Z

+        from fides.api.service.messaging.messaging_providers.mailgun_service import (
+            MailgunService,
+        )


should we do top level imports in this file too?

Done — moved all provider imports to module top level. Also checked all other PR files — no other local imports found.

erosselli · 2026-05-12T12:57:52Z

+    def test_all_service_types_have_providers(self):
+        assert set(_PROVIDER_MAP.keys()) == set(MessagingServiceType)


- Add RFC 2046 comment explaining multipart/alternative ordering - Extract header name constants on BaseEmailProviderService - Move test imports to module top level Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

…8122) Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Eliana Rosselli <67162025+erosselli@users.noreply.github.com>

JadeCara mentioned this pull request May 6, 2026

ENG-3301: Add correspondence template and reply-to utility (Story 3) #8123

Merged

8 tasks

Fix ruff import ordering and rename changelog to match PR #8118

ce9bc14

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

JadeCara force-pushed the ENG-3301/messaging-provider-refactor-pr2-ses branch from acc05e5 to ec08848 Compare May 6, 2026 18:10

JadeCara added a commit that referenced this pull request May 6, 2026

Add changelog entry for PR #8122

64bee10

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

JadeCara force-pushed the ENG-3301/messaging-provider-refactor-story2 branch from 916b01c to 64bee10 Compare May 6, 2026 18:15

JadeCara and others added 4 commits May 6, 2026 12:22

Add clarifying comments for incomplete provider map in PR1

57727f6

SES is intentionally excluded from _resolve_provider_map() and PROVIDER_MAP since it uses the legacy dispatcher until PR2. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Add changelog entry for PR #8120

aa85c56

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

JadeCara force-pushed the ENG-3301/messaging-provider-refactor-pr2-ses branch from ec08848 to b92ef93 Compare May 6, 2026 18:41

JadeCara and others added 3 commits May 6, 2026 12:51

Add changelog entry for PR #8122

8433b68

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Fix ruff formatting in test_provider_headers.py

f07ce63

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

JadeCara force-pushed the ENG-3301/messaging-provider-refactor-story2 branch from 64bee10 to f07ce63 Compare May 6, 2026 18:59

Base automatically changed from ENG-3301/messaging-provider-refactor-pr2-ses to main May 11, 2026 16:32

vercel Bot deployed to Preview – fides-plus-nightly May 11, 2026 17:54 View deployment

fixed changelog

1dd76b2

claude Bot reviewed May 11, 2026

View reviewed changes

JadeCara and others added 2 commits May 11, 2026 13:06

JadeCara marked this pull request as ready for review May 11, 2026 19:25

JadeCara requested a review from a team as a code owner May 11, 2026 19:25

JadeCara requested review from galvana and removed request for a team and galvana May 11, 2026 19:25

JadeCara requested a review from erosselli May 11, 2026 19:27

Fix Mailchimp header test for native reply_to field

a026b88

Test was asserting reply_to in headers dict, but we moved it to Mandrill's native msg_payload["reply_to"] field in the review feedback. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

erosselli reviewed May 11, 2026

View reviewed changes

JadeCara and others added 3 commits May 11, 2026 16:28

Update changelog/8122-threading-header-support.yaml

4691b97

Co-authored-by: Eliana Rosselli <67162025+erosselli@users.noreply.github.com>

Update src/fides/api/service/messaging/messaging_providers/aws_ses_se…

d6c47e1

…rvice.py Co-authored-by: Eliana Rosselli <67162025+erosselli@users.noreply.github.com>

erosselli approved these changes May 12, 2026

View reviewed changes

JadeCara and others added 2 commits May 12, 2026 10:14

Address PR review feedback

cfe132a

- Add RFC 2046 comment explaining multipart/alternative ordering - Extract header name constants on BaseEmailProviderService - Move test imports to module top level Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Merge branch 'main' into ENG-3301/messaging-provider-refactor-story2

1952a2c

vercel Bot deployed to Preview – fides-plus-nightly May 12, 2026 16:20 View deployment

JadeCara added this pull request to the merge queue May 12, 2026

Merged via the queue into main with commit 2f9f5e5 May 12, 2026
68 of 69 checks passed

JadeCara deleted the ENG-3301/messaging-provider-refactor-story2 branch May 12, 2026 17:31

		mail.content = Content("text/plain", body_text)
		mail.content = Content("text/html", message_body)

		def test_all_service_types_have_providers(self):
		assert set(_PROVIDER_MAP.keys()) == set(MessagingServiceType)

Conversation

JadeCara commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description Of Changes

Code Changes

Steps to Confirm

1. Server starts cleanly

2. Backward compatibility — send a test email

3. SES MIME migration

4. Threading header coverage (automated)

Pre-Merge Checklist

Uh oh!

vercel Bot commented May 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JadeCara commented May 11, 2026

Uh oh!

claude Bot left a comment

Choose a reason for hiding this comment

Code Review: PR #8122 — Threading header support for email providers

Bugs (latent — no caller sets body_text or threading fields yet)

Minor Suggestions

Positives

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erosselli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

JadeCara commented May 6, 2026 •

edited

Loading

vercel Bot commented May 6, 2026 •

edited

Loading

codecov Bot commented May 11, 2026 •

edited

Loading

Bugs (latent — no caller sets `body_text` or threading fields yet)